Ensure dispatch from `useFormState` works in `StrictMode` #28557

eps1lon · 2024-03-14T10:06:36Z

Summary

Closes #28556

We didn't follow the Rules of Hooks in the rerender implementation for mounts.

Test plan

Added test based on "useFormState updates state asynchronously and queues multiple actions" but with StrictMode and less dispatches (because it repros on the first attempt)

…tMode`

react-sizebot · 2024-03-14T10:09:57Z

Comparing: bbc571a...e32f38a

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	176.83 kB	176.84 kB	+0.02%	54.91 kB	54.92 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	173.55 kB	173.55 kB	=	54.11 kB	54.11 kB
facebook-www/ReactDOM-prod.classic.js	=	594.04 kB	594.07 kB	=	104.37 kB	104.38 kB
facebook-www/ReactDOM-prod.modern.js	=	577.30 kB	577.33 kB	=	101.42 kB	101.43 kB
test_utils/ReactAllWarnings.js	Deleted	66.87 kB	0.00 kB	Deleted	16.36 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
test_utils/ReactAllWarnings.js	Deleted	66.87 kB	0.00 kB	Deleted	16.36 kB	0.00 kB

Generated by 🚫 dangerJS against e32f38a

acdlite · 2024-03-19T16:04:13Z

packages/react-reconciler/src/ReactFiberHooks.js

@@ -2259,6 +2259,9 @@ function rerenderFormState<S, P>(
    );
  }

+  // For mount, pending is always false.
+  const [isPending] = rerenderState(false);


Since we don't actually need the result, all we need to do is advance the hook list.

Suggested change

const [isPending] = rerenderState(false);

updateWorkInProgressHook();

e32f38a (#28557)

Feels a bit brittle but you know this part better than I do.

I consider this pretty low level code so I think it's fine. I care more about not observing the value of isPending since it causes a bunch of redundant read operations.

packages/react-reconciler/src/ReactFiberHooks.js

acdlite · 2024-03-19T16:07:25Z

This was introduced by #28514. Easy oversight to make, glad we caught it quickly.

Note that it doesn't just affect StrictMode, the same issue happen if you called setState during render to trigger a re-render. Or if you suspended with use after useFormState has already been called. That last one is the most idiomatic so I'd probably write the regression test using that pattern.

Co-Authored-By: Andrew Clark <[email protected]>

Co-authored-by: Andrew Clark <[email protected]> DiffTrain build for [8ef14cf](8ef14cf)

…8557) Co-authored-by: Andrew Clark <[email protected]>

Co-authored-by: Andrew Clark <[email protected]> DiffTrain build for commit 8ef14cf.

Failing test for null dispatch returned from useFormState in `Stric…

c2f1a2c

…tMode`

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 14, 2024

eps1lon changed the title ~~Failing test for null dispatch returned from useFormState in StrictMode~~ Ensure dispatch in useFormState works in StrictMode Mar 14, 2024

eps1lon force-pushed the strictmode-useformstate branch from f792c17 to 1c4e7c4 Compare March 14, 2024 10:20

Ensure dispatch in useFormState works in StrictMode

9b3f1f3

eps1lon force-pushed the strictmode-useformstate branch from 1c4e7c4 to 9b3f1f3 Compare March 14, 2024 10:21

eps1lon marked this pull request as ready for review March 14, 2024 10:25

eps1lon requested a review from acdlite March 14, 2024 10:26

eps1lon changed the title ~~Ensure dispatch in useFormState works in StrictMode~~ Ensure dispatch from useFormState works in StrictMode Mar 14, 2024

eps1lon added the React 19 label Mar 14, 2024

ztanner mentioned this pull request Mar 18, 2024

Update React from 6c3b8dbfe to a4939017f vercel/next.js#63413

Closed

acdlite reviewed Mar 19, 2024

View reviewed changes

packages/react-reconciler/src/ReactFiberHooks.js Outdated Show resolved Hide resolved

Just advance the hook list

e32f38a

Co-Authored-By: Andrew Clark <[email protected]>

eps1lon requested a review from acdlite March 19, 2024 18:34

acdlite approved these changes Mar 20, 2024

View reviewed changes

eps1lon merged commit 8ef14cf into facebook:main Mar 20, 2024
38 checks passed

eps1lon deleted the strictmode-useformstate branch March 20, 2024 15:16

github-actions bot pushed a commit that referenced this pull request Mar 20, 2024

Ensure dispatch from useFormState works in StrictMode (#28557)

6e5eaa6

Co-authored-by: Andrew Clark <[email protected]> DiffTrain build for [8ef14cf](8ef14cf)

gnoff mentioned this pull request Mar 25, 2024

Update React from 14898b6a9 to e373190fa vercel/next.js#63685

Closed

EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024

Ensure dispatch from useFormState works in StrictMode (facebook#2…

7ace243

…8557) Co-authored-by: Andrew Clark <[email protected]>

bigfootjon pushed a commit that referenced this pull request Apr 18, 2024

Ensure dispatch from useFormState works in StrictMode (#28557)

ae299be

Co-authored-by: Andrew Clark <[email protected]> DiffTrain build for commit 8ef14cf.

eps1lon mentioned this pull request Apr 19, 2024

Update React from 14898b6a9 to c3048aab4 vercel/next.js#64798

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure dispatch from `useFormState` works in `StrictMode` #28557

Ensure dispatch from `useFormState` works in `StrictMode` #28557

eps1lon commented Mar 14, 2024 •

edited

Loading

react-sizebot commented Mar 14, 2024 •

edited

Loading

acdlite Mar 19, 2024

eps1lon Mar 19, 2024

eps1lon Mar 19, 2024

acdlite Mar 20, 2024

acdlite commented Mar 19, 2024

	const [isPending] = rerenderState(false);
	updateWorkInProgressHook();

Ensure dispatch from useFormState works in StrictMode #28557

Ensure dispatch from useFormState works in StrictMode #28557

Conversation

eps1lon commented Mar 14, 2024 • edited Loading

Summary

Test plan

react-sizebot commented Mar 14, 2024 • edited Loading

Critical size changes

Significant size changes

acdlite Mar 19, 2024

Choose a reason for hiding this comment

eps1lon Mar 19, 2024

Choose a reason for hiding this comment

eps1lon Mar 19, 2024

Choose a reason for hiding this comment

acdlite Mar 20, 2024

Choose a reason for hiding this comment

acdlite commented Mar 19, 2024

Ensure dispatch from `useFormState` works in `StrictMode` #28557

Ensure dispatch from `useFormState` works in `StrictMode` #28557

eps1lon commented Mar 14, 2024 •

edited

Loading

react-sizebot commented Mar 14, 2024 •

edited

Loading